Skip to content

add Type::isError method#4530

Open
canvural wants to merge 6 commits intophpstan:2.1.xfrom
canvural:is-error
Open

add Type::isError method#4530
canvural wants to merge 6 commits intophpstan:2.1.xfrom
canvural:is-error

Conversation

@canvural
Copy link
Contributor

@canvural canvural commented Nov 6, 2025

This one does not add too much value, but I just didn't want to do instanceof ErrorType in my extensions 😄

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApiInstanceofTypeRule should be made aware of this 😊

@ondrejmirtes
Copy link
Member

Do you also feel like doing isNever() + isExplicitNever()? :)

@canvural
Copy link
Contributor Author

canvural commented Nov 7, 2025

Do you also feel like doing isNever() + isExplicitNever()? :)

Didn't ever need that I guess, but sure I can. In a separate PR?

@canvural
Copy link
Contributor Author

canvural commented Nov 10, 2025

The mutations look valid, but I don't know how I can change them in bulk.

Edit: I guess $type instanceof ErrorType could be !$type->isError()->no() and !$type instanceof ErrorType can be $type->isError()->no() ?

@ondrejmirtes
Copy link
Member

I'm not sure how to cover the mutations with tests here. i'm more concerned about the things the issue bot found https://github.com/phpstan/phpstan-src/actions/runs/19215786400, I'm not sure why that's happening.

@staabm
Copy link
Contributor

staabm commented Nov 11, 2025

just a gut feeling in case the 2 above comments regarding behaviour changes are not valid:

another source of problems could be

  • class ErrorType extends MixedType
  • class TemplateMixedType extends MixedType

because MixedType - because of substraction logic - might lead to a Maybe.

Copy link
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #4547

$expressionType instanceof NeverType

by

!$expressionType->isNever()->no()

But here you're replacing

$type instanceof ErrorType

by

$type->isError()->yes()

How did you choose ? I feel like we should have consistency...

@canvural
Copy link
Contributor Author

In #4547

$expressionType instanceof NeverType

by

!$expressionType->isNever()->no()

But here you're replacing

$type instanceof ErrorType

by

$type->isError()->yes()

How did you choose ? I feel like we should have consistency...

Yeah we should have consistency. In the isNever PR I did it that way to see how the mutation testing will behave. But honestly I'd rather have $type->isError()->yes() everywhere, it's much easier to understand. So I'll change the other PR later.

@canvural canvural force-pushed the is-error branch 2 times, most recently from 08307bd to f46acaf Compare December 8, 2025 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments